-
-
Notifications
You must be signed in to change notification settings - Fork 14.4k
try enabling linker-messages by default again
#149937
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@bors try |
This comment has been minimized.
This comment has been minimized.
try enabling `linker-messages` by default again
This comment has been minimized.
This comment has been minimized.
|
@craterbot build-only |
|
🚨 Error: failed to parse the command 🆘 If you have any trouble with Crater please ask in t-infra on Zulip |
|
@craterbot run mode=build-only |
|
👌 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
don't use no_main and no_core to test IBT The previous test was quite fragile and depended on a bunch of internal features. Simplify it. Split out of rust-lang#149937. cc `@jieyouxu` `@Oneirical`
don't use no_main and no_core to test IBT The previous test was quite fragile and depended on a bunch of internal features. Simplify it. Split out of rust-lang#149937. cc ``@jieyouxu`` ``@Oneirical``
don't use no_main and no_core to test IBT The previous test was quite fragile and depended on a bunch of internal features. Simplify it. Split out of rust-lang#149937. cc ```@jieyouxu``` ```@Oneirical```
|
☔ The latest upstream changes (presumably #149999) made this pull request unmergeable. Please resolve the merge conflicts. |
don't use no_main and no_core to test IBT The previous test was quite fragile and depended on a bunch of internal features. Simplify it. Split out of rust-lang/rust#149937. cc ```@jieyouxu``` ```@Oneirical```
|
🚧 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
|
🎉 Experiment
Footnotes
|
- Hide common linker output behind `linker-info` - Add tests - Account for different capitalization on windows-gnu when removing "warning" prefix - Add some more comments - Add macOS deployment-target test - Ignore linker warnings from trying to statically link glibc I don't know what's going on in `nofile-limit.rs` but I want no part of it.
|
Some changes occurred in compiler/rustc_passes/src/check_attr.rs |
|
r? @mati865 |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
@zacknewman that warning has been moved into the new |
|
|
☔ The latest upstream changes (presumably #152063) made this pull request unmergeable. Please resolve the merge conflicts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple of questions regarding the tests, to make sure they don't cause issues for people working on their machines.
| fn is_windows_gnu_ld(sess: &Session) -> bool { | ||
| let (_, flavor) = linker_and_flavor(sess); | ||
| sess.target.is_like_windows | ||
| && !sess.target.is_like_msvc | ||
| && matches!(flavor, LinkerFlavor::Gnu(_, Lld::No)) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will match windows-gnu and windows-gnullvm targets. Probably doesn't matter enough to justify adding && sess.target.abi != Abi::Llvm there.
| @@ -0,0 +1,5 @@ | |||
| //@ only-windows-gnu | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait, this still happens on CI?
It should not since Sep 9, 2024: #119229 (comment)
Regardless of the CI, having a test that fails only with some host tools version doesn't seem right.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
um. hm. possibly this only fails for me locally because I have an old version of windows-gnu? I'm not sure how to test it in CI ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe instead I could add a fake-linker script that prints exactly the error message I see locally? does that seem reasonable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that's very likely. Does your ld -v show version older than 2.40?
Fake linker sounds fine for me, although that probably would have to be a batch script or powershell one since this test runs on Windows.
@bors try jobs=x86_64-mingw-1,x86_64-mingw-2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can test it on CI after you resolve conflicts.
@bors delegate
|
@rustbot author |
|
@rustbot review |
|
🔒 Merge conflict A merge attempt failed due to a merge conflict. Please rebase on top of the latest base How do I rebase?Assuming
You may also read Please avoid the "Resolve conflicts" button on GitHub. Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how |
|
@rustbot author |
linker-infofromlinker-messageslinker-messageswarn-by-default; keeplinker-infodeny-by-defaultHelps with #136096.